ROX-35289: add post-upgrade script to skip init container evaluation#123
ROX-35289: add post-upgrade script to skip init container evaluation#123AlexVulaj wants to merge 1 commit into
Conversation
|
Warning Review limit reached
More reviews will be available in 33 minutes and 3 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTwo new files are added under ChangesSkip Init Container Evaluation Utility
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@util-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh`:
- Line 22: The curl command uses the -k flag to disable TLS verification with
bearer token authentication, creating a significant security vulnerability that
allows MITM attacks. Replace all instances where curl is invoked with the -k
flag (appearing in the version assignment, and subsequent API calls throughout
the script) by gating the -k flag behind an explicit environment variable
opt-in. Create a conditional that only includes -k in the curl command when an
environment variable like SKIP_TLS_VERIFICATION is explicitly set to true or
similar value, ensuring TLS verification remains enabled by default while
allowing users to explicitly opt-in to insecure mode if required for their
environment.
- Around line 54-55: The script checks if skipContainerTypes already exists in
evaluationFilter to skip processing, but when updating the policy around line
70, it replaces the entire evaluationFilter object instead of merging with
existing fields. This causes loss of other filter fields in policies that
already have a different evaluationFilter. Modify the jq update operation to
merge the new skipContainerTypes with the existing evaluationFilter object using
jq's merge operator instead of replacing the entire evaluationFilter, ensuring
existing filter fields are preserved when the policy is updated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3f06df2e-4cba-4b89-9015-3569fb144fd3
📒 Files selected for processing (2)
util-scripts/skip-init-container-evaluation/README.mdutil-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh
| AUTH="Authorization: Bearer ${ROX_API_TOKEN}" | ||
|
|
||
| # Version check — require 5.0+ | ||
| version=$(curl -sk -H "$AUTH" "$API/v1/metadata" | jq -r '.version') |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Bearer-token API calls disable TLS verification.
Using curl -k for authenticated requests allows MITM interception/tampering on the connection path. For a policy-mutation script, this is a significant security risk.
Suggested fix
-version=$(curl -sk -H "$AUTH" "$API/v1/metadata" | jq -r '.version')
+version=$(curl -sS --fail -H "$AUTH" "$API/v1/metadata" | jq -r '.version')
@@
-policies=$(curl -sk -H "$AUTH" "$API/v1/policies" | jq -r '.policies[].id')
+policies=$(curl -sS --fail -H "$AUTH" "$API/v1/policies" | jq -r '.policies[].id')
@@
- policy=$(curl -sk -H "$AUTH" "$API/v1/policies/$id")
+ policy=$(curl -sS --fail -H "$AUTH" "$API/v1/policies/$id")
@@
- result=$(curl -sk -o /dev/null -w "%{http_code}" -XPUT -H "$AUTH" -H "Content-Type: application/json" \
+ result=$(curl -sS --fail -o /dev/null -w "%{http_code}" -XPUT -H "$AUTH" -H "Content-Type: application/json" \
"$API/v1/policies/$id" --data "$updated_policy")If insecure TLS is required for some environments, gate it behind an explicit opt-in env var instead of defaulting to -k. As per path instructions, this is a major security issue to prioritize.
Also applies to: 33-33, 50-50, 72-73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@util-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh`
at line 22, The curl command uses the -k flag to disable TLS verification with
bearer token authentication, creating a significant security vulnerability that
allows MITM attacks. Replace all instances where curl is invoked with the -k
flag (appearing in the version assignment, and subsequent API calls throughout
the script) by gating the -k flag behind an explicit environment variable
opt-in. Create a conditional that only includes -k in the curl command when an
environment variable like SKIP_TLS_VERIFICATION is explicitly set to true or
similar value, ensuring TLS verification remains enabled by default while
allowing users to explicitly opt-in to insecure mode if required for their
environment.
Source: Path instructions
| existing_filter=$(echo "$policy" | jq -r '.evaluationFilter.skipContainerTypes // empty') | ||
| if [[ -n "$existing_filter" ]]; then |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Skip logic and update payload can overwrite existing evaluation filters.
The script only skips when evaluationFilter.skipContainerTypes exists, then replaces the whole evaluationFilter object. Policies that already have a different evaluationFilter will be mutated and lose existing filter fields.
Suggested fix
- # Skip if already has skipContainerTypes set
- existing_filter=$(echo "$policy" | jq -r '.evaluationFilter.skipContainerTypes // empty')
+ # Skip if any evaluation filter is already configured
+ existing_filter=$(echo "$policy" | jq -r '.evaluationFilter // empty')
if [[ -n "$existing_filter" ]]; then
- echo " SKIP: \"$name\" — already has container type filter"
+ echo " SKIP: \"$name\" — already has evaluation filter"
skipped=$((skipped + 1))
continue
fi
@@
- updated_policy=$(echo "$policy" | jq '.evaluationFilter = {"skipContainerTypes": ["INIT"]}')
+ updated_policy=$(echo "$policy" | jq '.evaluationFilter = {"skipContainerTypes": ["INIT"]}')As per path instructions, this is a major correctness/maintainability issue to prioritize.
Also applies to: 70-70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@util-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh`
around lines 54 - 55, The script checks if skipContainerTypes already exists in
evaluationFilter to skip processing, but when updating the policy around line
70, it replaces the entire evaluationFilter object instead of merging with
existing fields. This causes loss of other filter fields in policies that
already have a different evaluationFilter. Modify the jq update operation to
merge the new skipContainerTypes with the existing evaluationFilter object using
jq's merge operator instead of replacing the entire evaluationFilter, ensuring
existing filter fields are preserved when the policy is updated.
Source: Path instructions
AlexVulaj
force-pushed
the
AlexVulaj/ROX-35289-skip-init-container-script
branch
from
9d37eb4 to
52dd31f
Compare
1 participant
Description
Adds a post-upgrade script for customers upgrading to ACS 5.0+ who want to preserve the pre-5.0 behavior where init containers were not evaluated by policies.
Starting in 5.0, policies evaluate init containers by default. This script adds
skipContainerTypes: ["INIT"]to all existing policies that don't already have an evaluation filter, effectively opting out of init container evaluation on a per-policy basis.Features:
Tested against a live ACS deployment.
Policy-as-Code users are directed to update their CRD manifests directly instead of running the script (see README).